-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BBA/HLE: Code refactoring #12559
BBA/HLE: Code refactoring #12559
Conversation
class NetworkRef | ||
{ | ||
public: | ||
StackRefs& data(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since these are super basic, it's probably fine to implement these inline.
Also it might be nice to also provide const and non-const begin()
/end()
member functions so you can iterate without needing to get the iterator off data()
, which would be a little more idiomatic.
So rather than:
for (auto& net_ref : m_network_ref.data())
you could just do:
for (auto& net_ref : m_network_ref)
dff39ae
to
405ca1e
Compare
This can now be rebased, the PRs it's depending on are merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly right to me, the one part that isn't obvious to me if it changes behavior or not is 6e2a081, because it swaps the order of the two operations.
The behaviour should be unchanged as the sleeping/enqueued data will be handled later (on the next iteration) after processing the socket data. |
I'll just believe you on that, it seems fine either way. |
This PR is based on #12533 and #12555. I needed to refactor it to deduplicate parts of the code and to allow some parts to be easier to modify in the future.
It shouldn't have any functional change.